Code review
CL とは Change Lists のことで、変更点くらいの意味だろう
A key point here is that there is no such thing as “perfect” code—there is only better code. Reviewers should not require the author to polish every tiny piece of a CL before granting approval.
Mostly, we expect developers to test CLs well-enough that they work correctly by the time they get to code review.
In the general case, look at every line of code that you have been assigned to review.
100 lines is usually a reasonable size for a CL, and 1000 lines is usually too large, but it’s up to the judgment of your reviewer.
レビュー修正が溜まって commit が増えてしまう問題(?) をどうするべきか
fixup を使う
レビューされた後に commit をいじることに不安を覚えるがどうだろうか?
OSS でもマージする前に squash するプロジェクトとかある気がするから別にいいのか
自分の考え
なるべく早く Draft PR を作成
リリースしても良いと思っているコードしかレビュー依頼を出さない
つまりサーバーでの動作確認ができていないコードはレビューにはこない
PR の説明では reviewer に必要な情報をすべて与える
例えば
PR の意図はコードから読み取れないことが多いので説明する
新しく導入するパッケージがある場合はリンク貼る
reviewer が詳しくないと事前に予想がついていることについては参考になるリンクを貼る
特にチェックして欲しいポイント、意見が欲しいポイントがある場合は書いておく
別 PR で対応するものがある場合は書いておく
レビューの最中に不要な force-push をしない
reviewer がローカルに pull してくるのがちょっと手間になる
マージは PR 作成者が行う